[fix][core] restrict export projections to include and exclude (24.05) - #7946
Conversation
/o/export/db parses the caller's projection and hands it to find() as given. MongoDB 4.4
and later evaluate aggregation expressions in a find projection, so a value like
{"pw": "$password"} renames a field rather than selecting one, and the credential redaction
that runs afterwards removes fields by name. A renamed field therefore carries its value
into the export under a name the redaction does not know. The same applies to computed
expressions, and $function evaluates javascript inside the database engine.
The DB Viewer already guarded its own projections for exactly this reason, and its comment
names the vector. That guard was never applied to the export path, even though the commit
that added the export redaction set out to align the two.
Move the guard into api/utils/common.js so there is one implementation, have the DB Viewer
helper delegate to it, and apply it to the export path after the projection is parsed.
Plain include and exclude are untouched, which is everything the dashboard sends: the
datatable exports pass field lists of 0 and 1.
# Conflicts: # plugins/dbviewer/api/parts/query_guard.js
Conflict resolved
Kept the base branch's
That is the reject-never-strip rule the rest of the query guards follow, so the base is right and the wrapper on this side was the older shape. What was dropped is small: a thin 61 passing across One thing for you to decide, which I did not change. The dbviewer path now rejects a bad projection while the export path still strips it. The comment above argues a silent strip is the wrong behaviour, and that argument applies equally to exports — an export whose projection was quietly altered returns different data than asked for. If that was a deliberate split, fine; if not, it is worth making |
2 participants
Backport of #7945 to release.24.05.
What
/o/export/dbparses the caller'sprojectionand passes it tofind()unchanged:MongoDB 4.4 and later evaluate aggregation expressions in a find projection, so a value can rename or compute a field rather than select one. The credential redaction that runs on the cursor removes fields by name, so a renamed field carries its value into the export file under a name the redaction does not know about.
Measured against MongoDB 7.0.37, all of these are accepted by
find()and return the underlying values:{"pw": "$password", "ak": "$api_key", "tfa": "$two_factor_auth"}{"secret": "$two_factor_auth.secret_token"}{"both": {"$concat": ["$password", "$api_key"]}}{"x": {"$function": {"body": "...", "lang": "js"}}}The DB Viewer already guarded its own projections for this exact reason, and its comment names the vector, including the
$functioncase. The guard was never applied to the export path, although the commit that added the export redaction set out to align the two.Change
common.sanitizeProjectioninapi/utils/common.js: one implementation, restricting a projection to0,1,trueandfalse.plugins/dbviewer/api/parts/query_guard.jsdelegates to it, so the DB Viewer and the export cannot drift apart again.Scope
Every place a caller influenced projection can reach a database, across the three repositories:
/o/export/db->exports.fromDatabase->find(query, {projection})/o/export/request->exports.fromRequestplugins/views$projectpipelinesplugins/surveysaggregationoptions.projectNot changed, on purpose
sortandformatFieldson the same handler are also parsed from the request. A sort value is not an expression context in the same way, and the dashboard sends real sort objects, so tightening them here would risk breaking exports for no security gain.plugins/dbviewer/api/api.js) takes a caller supplied object into a ClickHouse query builder. Mongo style expressions do not apply there, so this guard is not the right tool, and whether that builder is safe with arbitrary keys is a separate question worth its own look rather than a change made in passing.Verification
test/unit-tests/api.utils.common.jsgains 6 cases: plain include and exclude survive untouched, field path aliases and nested aliases are dropped,$concat,$functionand$condare dropped, invalid values such as2,NaNand strings are dropped, and a missing or non object projection is handled. Replacing the guard with a pass through fails 4 of them.release.24.05three unrelated cases in that file already fail on stock (mongodb.ObjectID is not a function, a driver artifact). The 6 added cases pass there.